Skip to content

refactor(tooling): replace SQLite skill tracking with CSV log and Cursor slash-command hook#29139

Open
NicolasMassart wants to merge 21 commits into
mainfrom
MCWP-513-po-c-phase-2-automated-collection-followup
Open

refactor(tooling): replace SQLite skill tracking with CSV log and Cursor slash-command hook#29139
NicolasMassart wants to merge 21 commits into
mainfrom
MCWP-513-po-c-phase-2-automated-collection-followup

Conversation

@NicolasMassart
Copy link
Copy Markdown
Contributor

@NicolasMassart NicolasMassart commented Apr 21, 2026

Description

Follow-up to #28815 (Phase 2 automated collection).

Rewrites the AI tool usage collection system to use a local CSV append log instead of direct SQLite writes from hooks. The root cause driving the change: the hook shell environment does not honour .nvmrc, so hooks were running against the system Node rather than the project's pinned version — making better-sqlite3 bindings unreliable. The fix decouples write-time from read-time: hooks now do a pure-shell printf >> file (zero Node dependency), and the SQLite DB will be populated on-demand by dev-tooling-explorer or the future cron.

What changed:

  • Architecture: Hooks now append one CSV row to ~/.tool-usage-collection/metamask-mobile-events.log. The SQLite DB is populated on-demand by dev-tooling-explorer or the nightly cron — not by the hooks themselves.
  • Pure-shell hooks: Replaced all Node/tsx/SQLite hook infrastructure with two tiny shell entry scripts (hook-cursor-dispatch.sh, hook-claude-dispatch.sh) and a shared hook-common.sh. No Node, no yarn, no subprocess spawning in the hook path.
  • Split dispatchers: Cursor and Claude each have their own entry script. hook-cursor-dispatch.sh unconditionally emits {"permission":"allow"} as its first line (before any CI/opt-out check) so tool use is never blocked regardless of tracking state.
  • Cursor slash-command tracking (Path 4): Added hook-cursor-prompt-dispatch.sh on the beforeSubmitPrompt event. When a user invokes a skill via a slash command (e.g. /mms-pr-changelog), Cursor injects the skill content directly into the prompt context without the agent reading a SKILL.md file — so the existing preToolUse path never fires. This hook intercepts the raw prompt, extracts the skill name, and appends one CSV row. The {"permission":"allow"} response is always emitted first.
  • Fast-exit guard: The CI/opt-out guard runs before reading stdin in both hook-common.sh and hook-cursor-prompt-dispatch.sh — no unnecessary cat on opted-out runs.
  • False-positive fix: Path-based skill detection is now gated on the tool being a file-read tool (ReadFile/Read) — prevents StrReplace and other tools carrying skill paths as string content from generating spurious log entries.
  • CSV header: Log files are self-describing — a header row is written on first creation.
  • .claude/settings.json: Project-level PreToolUse hook on the Skill tool (replaces per-skill frontmatter hooks). Zero per-skill boilerplate.
  • Removed: db.ts, events.ts, tool-usage-collection.ts, cursor-hook-skill-tracking.ts, better-sqlite3 dependency, @types/better-sqlite3.
  • Tests: Full Jest coverage for all three dispatcher scripts (via child_process.execFileSync) and the Yarn plugin's CSV append path. Test file renamed from hook-skill-tracking-dispatch.test.ts to hook-dispatchers.test.ts to match the actual scripts.
  • Docs: Updated scripts/tooling/README.md with new architecture diagram (all 4 collection paths) and AGENTS.md collection-path table.

Changelog

CHANGELOG entry: null

Related issues

Refs: MCWP-513

Manual testing steps

Feature: CSV log-based skill tracking for Cursor and Claude

  Background:
    Given the branch is checked out and dependencies are installed with `yarn install`
    And Claude Code and Cursor are configured with this repo as the working directory
    And Common skills are installed from https://github.com/Consensys/skills

  Scenario: Cursor hook tracks a skill activation via agent read and never blocks tool use
    Given the log file does not exist at ~/.tool-usage-collection/metamask-mobile-events.log
    When the user prompts Cursor with "what's the changelog for this PR?"
    Then a new row appears in the log with tool_name="skill:mms-pr-changelog" and agent_vendor="cursor"
      # verify: tail -5 ~/.tool-usage-collection/metamask-mobile-events.log
    And Cursor tool use was not blocked

  Scenario: Cursor slash-command tracking via beforeSubmitPrompt hook
    When user types "/mms-pr-changelog" in a Cursor chat and submits it
    Then a new row appears in the log with tool_name="skill:mms-pr-changelog" and agent_vendor="cursor"
    # verify: tail -5 ~/.tool-usage-collection/metamask-mobile-events.log
    And the prompt submission was not blocked

  Scenario: Cursor hook still emits allow when CI is set or user is opted out
    Given CI=true is exported in the shell (or TOOL_USAGE_COLLECTION_OPT_IN=false)
    When the user prompts Cursor with "what's the changelog for this PR?"
    Then no new rows are written to the log
    # verify: tail -5 ~/.tool-usage-collection/metamask-mobile-events.log
    And the hook exits 0 without blocking Cursor

  Scenario: Claude Code global hook tracks a skill activation
    Given CI and TOOL_USAGE_COLLECTION_OPT_IN are reset/removed since previous test
    When user starts a Claude Code session
    And the user prompts Claude with "what's the changelog for this PR?"
    Then a new row appears with tool_name="skill:mms-pr-changelog" and agent_vendor="claude"
    # verify: tail -5 ~/.tool-usage-collection/metamask-mobile-events.log

  Scenario: Yarn script tracking
    When user runs `yarn lint`
    Then start and end rows appear in the log for that script
    # verify: tail -5 ~/.tool-usage-collection/metamask-mobile-events.log

  Scenario: Yarn script tracking interrupted
    When user runs `yarn lint`
    And cancels the lint run with `ctrl+C`
    Then start and interrupted rows appear in the log for that script
    # verify: tail -5 ~/.tool-usage-collection/metamask-mobile-events.log

Screenshots/Recordings

Before

N/A

After

Log file should look like the following:

$ tail -5 ~/.tool-usage-collection/metamask-mobile-events.log
tool_name,tool_type,event_type,agent_vendor,session_id,success,duration_ms,created_at
skill:mms-pr-changelog,skill,start,cursor,sess-123,,,2026-05-04T13:37:10Z
skill:mms-pr-changelog,skill,start,cursor,sess-456,,,2026-05-04T13:37:42Z
skill:mms-pr-changelog,skill,start,claude,sess-789,,,2026-05-04T13:37:51Z
yarn:lint,yarn_script,start,,,,,2026-05-04T13:38:10Z
yarn:lint,yarn_script,end,,,,120000,2026-05-04T13:40:10Z
yarn:lint,yarn_script,start,,,,,2026-05-04T13:50:10Z
yarn:lint,yarn_script,interrupted,,,,5000,2026-05-04T13:50:20Z

Pre-merge author checklist

Performance checks (if applicable)

  • I've tested on Android
    • Ideally on a mid-range device; emulator is acceptable
  • I've tested with a power user scenario
    • Use these power-user SRPs to import wallets with many accounts and tokens
  • I've instrumented key operations with Sentry traces for production performance metrics

For performance guidelines and tooling, see the Performance Guide.

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Note

Medium Risk
Touches developer workflow hooks for Yarn/Claude/Cursor; a bug could silently stop tracking or (for Cursor) potentially interfere with tool/prompt flow despite safeguards to always emit allow.

Overview
Replaces SQLite-based tool/skill usage tracking with a local append-only CSV log (~/.tool-usage-collection/metamask-mobile-events.log) and removes the Node/SQLite CLI + better-sqlite3 dependency.

Updates hook wiring to use pure-shell dispatchers: adds .claude/settings.json project PreToolUse hook, changes Cursor to preToolUse + new beforeSubmitPrompt hook for slash-command skills, and adds shared hook-common.sh to extract skill names/session ids and append CSV with a one-time header.

Refactors the Yarn Berry plugin to write CSV rows directly (start/end/interrupted with duration/success) and adds Jest coverage for the plugin and new shell dispatchers; updates docs/config (AGENTS.md, scripts/tooling/README.md, babel.config.tests.js, .gitignore) accordingly.

Reviewed by Cursor Bugbot for commit 3b1e8d1. Bugbot is set up for automated code reviews on this repo. Configure here.

@NicolasMassart NicolasMassart self-assigned this Apr 21, 2026
@github-actions
Copy link
Copy Markdown
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@metamaskbotv2 metamaskbotv2 Bot added the team-mobile-platform Mobile Platform team label Apr 21, 2026
@github-actions
Copy link
Copy Markdown
Contributor

E2E Fixture Validation — Schema is up to date
12 value mismatches detected (expected — fixture represents an existing user).
View details

…oach

Rewrites the AI tool usage collection system to use a local CSV append
log instead of a direct SQLite write from hooks. Hooks are now pure-shell
(no Node/yarn/tsx dependency), with the DB populated on-demand by the
dev-tooling-explorer or nightly cron. Splits the shared dispatcher into
agent-specific entry scripts (hook-cursor-dispatch.sh, hook-claude-dispatch.sh)
and a common script (hook-common.sh). Removes obsolete Node modules
(db.ts, events.ts, tool-usage-collection.ts, cursor-hook-skill-tracking.ts)
and the better-sqlite3 dependency. Adds full test coverage for both the
shell dispatcher and the Yarn plugin.

Co-authored-by: Cursor <cursoragent@cursor.com>
@NicolasMassart NicolasMassart force-pushed the MCWP-513-po-c-phase-2-automated-collection-followup branch from 18aeef3 to 300187f Compare May 12, 2026 13:34
@github-actions github-actions Bot added size-XL and removed size-L labels May 12, 2026
@github-actions
Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: None (no tests recommended)
  • Selected Performance tags: None (no tests recommended)
  • Risk Level: low
  • AI Confidence: 97%
click to see 🤖 AI reasoning details

E2E Test Selection:
All 21 changed files are developer tooling infrastructure with no impact on the MetaMask Mobile app:

  1. .yarn/plugins/plugin-usage-tracking.cjs - Yarn Berry plugin refactored from SQLite-based to CSV-based event logging. Pure developer tooling, not app code.
  2. .yarn/plugins/plugin-usage-tracking.test.ts - Tests for the above plugin.
  3. .claude/settings.json / .claude/skills/pr-changelog/SKILL.md - Claude AI assistant configuration changes (removing PreToolUse hooks from skill frontmatter).
  4. .cursor/hooks.json - Cursor IDE hook configuration.
  5. scripts/tooling/ (README.md, *.ts, *.sh, *.test.ts files) - Developer usage collection scripts refactored from SQLite to CSV-based logging. These are internal developer tools never executed in CI E2E tests.
  6. AGENTS.md - Documentation only.
  7. babel.config.tests.js - Only adds two new tooling files (.yarn/plugins/plugin-usage-tracking.cjs and .yarn/plugins/plugin-usage-tracking.test.ts) to the transform-inline-environment-variables exclusion list. This is a purely additive, non-breaking change that won't affect any existing test compilation.
  8. package.json - Flagged as critical but the actual diff shows only tooling-related script changes consistent with the refactoring above.

None of these changes touch: app components, controllers, navigation, Engine, test fixtures, E2E test page objects, or any code path exercised by Detox E2E tests. No E2E tags are warranted.

Performance Test Selection:
No app code, UI components, controllers, or performance-sensitive paths were modified. All changes are developer tooling infrastructure (usage tracking, AI agent hooks, documentation). No performance impact on the app.

View GitHub Actions results

@NicolasMassart NicolasMassart added Code Impact - Low Minor code change that can safely applied to the codebase area-devex Issues and PRs focused on developer experience skip-e2e skip E2E test jobs labels May 12, 2026
Changed the error type from `NodeJS.ErrnoError` to `NodeJS.ErrnoException` in the `runDispatcher` function to ensure proper error handling in the test suite.
…prompts

- Introduced a new `beforeSubmitPrompt` hook in `.cursor/hooks.json` that triggers the `hook-cursor-prompt-dispatch.sh` script.
- The new script processes user prompts, extracts skill names from slash commands, and logs relevant data while ensuring prompt submissions are never blocked.
- Updated `hook-common.sh` to improve payload handling.
- Added comprehensive tests for the new functionality in `hook-dispatchers.test.ts`.
- Updated documentation in `README.md` to reflect the new hook and its purpose.
@NicolasMassart NicolasMassart changed the title refactor(tooling): unify Claude skill tracking into a single global hook refactor(tooling): replace SQLite skill tracking with CSV log and Cursor slash-command hook May 12, 2026
NicolasMassart and others added 2 commits May 12, 2026 20:22
… github.com:MetaMask/metamask-mobile into MCWP-513-po-c-phase-2-automated-collection-followup
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 43.21%. Comparing base (3751d9a) to head (1e56de5).
⚠️ Report is 39 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (3751d9a) and HEAD (1e56de5). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (3751d9a) HEAD (1e56de5)
2 1
Additional details and impacted files
@@             Coverage Diff             @@
##             main   #29139       +/-   ##
===========================================
- Coverage   81.54%   43.21%   -38.34%     
===========================================
  Files        5343     5358       +15     
  Lines      142128   142541      +413     
  Branches    32411    32527      +116     
===========================================
- Hits       115899    61595    -54304     
- Misses      18299    75110    +56811     
+ Partials     7930     5836     -2094     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

NicolasMassart and others added 3 commits May 13, 2026 10:51
- Changed the execution method from `.` to `sh` for sourcing `hook-common.sh` in both `hook-claude-dispatch.sh` and `hook-cursor-dispatch.sh` scripts to ensure proper script execution.
… github.com:MetaMask/metamask-mobile into MCWP-513-po-c-phase-2-automated-collection-followup
@NicolasMassart NicolasMassart marked this pull request as ready for review May 13, 2026 09:35
@github-project-automation github-project-automation Bot moved this to Needs dev review in PR review queue May 13, 2026
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fc9674fe4a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread scripts/tooling/hook-cursor-prompt-dispatch.sh
NicolasMassart and others added 4 commits May 13, 2026 13:08
- Introduced a guard to log only known skills by checking for the existence of SKILL.md files in predefined directories.
- This prevents misclassification of built-in Cursor commands as skill invocations, enhancing the accuracy of skill handling in the script.
… github.com:MetaMask/metamask-mobile into MCWP-513-po-c-phase-2-automated-collection-followup
- Added a `beforeEach` hook to create a temporary skills directory and a stub `SKILL.md` file for testing slash command invocations.
- Updated environment variables in test cases to ensure proper resolution of the temporary directory for skill commands.
- This change improves the accuracy of tests related to slash command handling in the dispatcher.
NicolasMassart and others added 2 commits May 15, 2026 12:59
- Updated the plugin usage tracking to clarify that only Yarn scripts defined in package.json are logged, addressing a Yarn Berry limitation.
- Improved the skill name validation in hook-cursor-prompt-dispatch.sh to allow for underscores in skill names, ensuring better compatibility with various naming conventions.
- Refactored log file creation to prevent duplicate header rows when multiple scripts run concurrently, enhancing the reliability of the logging mechanism.
- Updated tests to ensure correct logging behavior for known and unknown skills, improving overall test coverage and accuracy.
andrepimenta
andrepimenta previously approved these changes May 15, 2026
@github-project-automation github-project-automation Bot moved this from Needs dev review to Review finalised - Ready to be merged in PR review queue May 15, 2026
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 5126c91. Configure here.

Comment thread scripts/tooling/hook-dispatchers.test.ts
… update test environment variables

- Updated the skill logging guard in `hook-cursor-prompt-dispatch.sh` to allow overriding the repository root for testing purposes.
- Modified test cases in `hook-dispatchers.test.ts` to use the new `TOOL_USAGE_REPO_ROOT` environment variable instead of `GIT_DIR`, improving test isolation and flexibility.
@sonarqubecloud
Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-devex Issues and PRs focused on developer experience Code Impact - Low Minor code change that can safely applied to the codebase size-XL skip-e2e skip E2E test jobs team-mobile-platform Mobile Platform team

Projects

Status: Review finalised - Ready to be merged

Development

Successfully merging this pull request may close these issues.

3 participants